-
Notifications
You must be signed in to change notification settings - Fork 55
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat/test for one time sbt.sol #217
Conversation
motemotech
commented
Oct 8, 2024
•
edited
Loading
edited
- Change OneTimeSBT contract
- Add Test for OneTimeSBT.sol
- Add coverage tool to hardhat.config.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added my comment for all changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the same constants which is registered in the contract
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the file to generate mock proof for verifier contract when I run test code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small changes, include bug fix
@@ -29,7 +29,7 @@ contract OneTimeSBT is ERC721Enumerable { | |||
error UNEQUAL_BLINDED_DSC_COMMITMENT(); | |||
error INVALID_PROVE_PROOF(); | |||
error INVALID_DSC_PROOF(); | |||
error SBT_CAN_BE_TRANSFERED(); | |||
error SBT_CAN_NOT_BE_TRANSFERED(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not correct error message
@@ -49,17 +49,17 @@ contract OneTimeSBT is ERC721Enumerable { | |||
// Convert the last four parameters into a valid timestamp, adding 30 years to adjust for block.timestamp starting in 1970 | |||
uint[6] memory dateNum; | |||
for (uint i = 0; i < 6; i++) { | |||
dateNum[i] = p_proof.pubSignals[PROVE_RSA_COMMITMENT_INDEX + i]; | |||
dateNum[i] = p_proof.pubSignals[PROVE_RSA_CURRENT_DATE_INDEX + i]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not correct index place
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to read older than from revealedData_Packed anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is needed for SBT.sol function.
I just put my compilation result in my local here, but we can change this after deploy in production
contracts/hardhat.config.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add coverage tool
contracts/test/OneTimeSBT.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test code for OneTimeSBT.sol
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is outdated too much and I need to comment out all of this to run coverage test